Thanks for the pull request, @rpenido!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/2u-tnl. Tag them in a comment and let them know that your changes are ready for review.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

Codecov Report

Attention: Patch coverage is 95.96774% with 5 lines in your changes missing coverage. Please review.

Project coverage is 93.08%. Comparing base (64ffadd) to head (68ac667).
Report is 1 commits behind head on master.

@@            Coverage Diff             @@
##           master    #1217      +/-   ##
==========================================
+ Coverage   93.06%   93.08%   +0.02%     
==========================================
  Files         756      760       +4     
  Lines       13777    13890     +113     
  Branches     2984     3007      +23     
==========================================
+ Hits        12822    12930     +108     
- Misses        919      923       +4     
- Partials       36       37       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Did you intentionally change the width of the "Library Info Sidebar"? Now it seems too wide.

Yes.. I was trying to make it wider in the 1200-1320 range. I added a maximum width now to prevent it from going wider now: 3c7007e

Did you make the sidebar so it no longer overlaps with the header? That may be OK but we should clear it with the UX team if it wasn't their idea. The way that the "user widget" in the header is now randomly aligned with the sidebar based on the user's resolution looks bad in my opinion. CC @lizc577 @sdaitzman

Yes. I changed it to match the Figma design.

I didn't find a discussion about it, but I agree that the alignment looks odd (because header have a max xl size). Putting our container (content + sidebar) at a max width of xl will give us a better alignment, but we would lose a lot of screen space.

^ On a large monitor, the sidebar is too big and the buttons look terrible. We should have a fixed width for the sidebar above a certain size. It shouldn't get this wide, I think.

Added a max width 3c7007e

I think the buttons should always stretch to fill the full width, not have these gaps between them

Fixed

On small screens, or when the text is translated into another language (e.g. Russian) where the description is longer, the buttons currently get "fat" (see above). I think it would be better to wrap them like this

I also changed this

I still need to fix the responsiveness for small resolutions, but I will wait for the design team before tinkering with it.

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@rpenido @bradenmacdonald thanks for checking in about these UX issues.

1. Did you intentionally change the width of the "Library Info Sidebar"? Now it seems too wide.

In the screenshot, the sidebar looks far too wide. @lizc577 may have more specific constraints, but I think something around 1000px as a maximum size makes sense. Maybe even smaller?

2. Did you make the sidebar so it no longer overlaps with the header? That may be OK but we should clear it with the UX team if it wasn't their idea. The way that the "user widget" in the header is now randomly aligned with the sidebar based on the user's resolution looks bad in my opinion.

Is it possible to make the header full width and vertically aligned with the rest of the page, e.g. place the right page header elements vertically in line with the right sidebar elements? This is an example from Figma with lots of placeholder elements, let me know if a more accurate example with the real header elements would be helpful.

3. The white background on the sidebar is nice though.

Agreed. This is inverted from the designs in Figma, but it provides the intended slight visual separation.

4. ^ On a large monitor, the sidebar is too big and the buttons look terrible. We should have a fixed width for the sidebar above a certain size. It shouldn't get this wide, I think.

Agreed, looks too wide to me. Thanks for fixing this @rpenido.

5. I think the buttons should always stretch to fill the full width, not have these gaps between them:

Agreed, I think full width buttons make sense.

6. On small screens, or when the text is translated into another language (e.g. Russian) where the description is longer, the buttons currently get "fat" (see above). I think it would be better to wrap them like this:

That wrapping makes sense to me rather than showing multiple lines of text in one button, when possible to avoid that. The overflow menu ... icon position is a bit strange, but I think it's okay for now.

@rpenido That looks much better - thanks!

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Thanks @rpenido the buttons look better.

1. Did you intentionally change the width of the "Library Info Sidebar"? Now it seems too wide.

In the screenshot, the sidebar looks far too wide. @lizc577 may have more specific constraints, but I think something around 1000px as a maximum size makes sense. Maybe even smaller?

2. Did you make the sidebar so it no longer overlaps with the header? That may be OK but we should clear it with the UX team if it wasn't their idea. The way that the "user widget" in the header is now randomly aligned with the sidebar based on the user's resolution looks bad in my opinion.

Is it possible to make the header full width and vertically aligned with the rest of the page, e.g. place the right page header elements vertically in line with the right sidebar elements? This is an example from Figma with lots of placeholder elements, let me know if a more accurate example with the real header elements would be helpful.

@rpenido I saw the same problems, Is there anything that can be done?

In the screenshot, the sidebar looks far too wide. @lizc577 may have more specific constraints, but I think something around 1000px as a maximum size makes sense. Maybe even smaller?

Now I'm using the xs breakpoint (extra small) as maximum size, which is 464px (https://paragon-openedx.netlify.app/foundations/layout/#max-width). Let me know what you think!

Is it possible to make the header full width and vertically aligned with the rest of the page, e.g. place the right page header elements vertically in line with the right sidebar elements? This is an example from Figma with lots of placeholder elements, let me know if a more accurate example with the real header elements would be helpful.

It is possible, but we should try to be consistent throughout the app.

Currently, all pages (with some minor padding/margin issues 😅 ) have a maximum width in the xl breakpoint (1440px), the same size as the maximum header and footer.

Studio Home

Course Outline

Library Home (WITHOUT sidebar)

The Library Home (WITH sidebar) is currently breaking this rule, hence, it looks not aligned

Staying inside the xl breakpoint would make the whole app look more consistent.
We will have a sidebar that doesn't come from the page's edge.

@sdaitzman, is it possible to tweak the sidebar to make it look bet,ter following the max xl rule, like in the last screenshot?
I don't think it's the better solution, but should I keep it the way it was before (not wrapped by the header/footer)?

CC @ChrisChV @bradenmacdonald

It is possible, but we should try to be consistent throughout the app.

Currently, all pages (with some minor padding/margin issues 😅 ) have a maximum width in the xl breakpoint (1440px), the same size as the maximum header and footer.

Thanks @rpenido! I can propose some sidebar design variants to avoid the staggered right vertical, but I would personally support slightly modifying the header within content libraries to fix the alignment issue. The header's contents would stay the same, just positioned to fill the screen and line up with the content libraries layout.

I think this potentially also interacts with #1221. Currently, components and collections are limited to four columns with wide gutters on large viewports. I think that users will benefit from using their full screen area when managing content libraries, and that it will look strange if the header is extremely narrow once the cards take up the full space.

@rpenido @sdaitzman Feel free to design whatever you'd like as a long-term solution, but I'd like to keep any changes to the header outside of the scope of our MVP. For now, having the sidebar as it already is on the sandbox (see below) avoids any issues - the header stays aligned with the content outside of the sidebar, and the sidebar has maximum real estate.

Changed the sidebar position and fixed responsiveness

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@rpenido That looks good to me - nice work! And thanks for the animation, that's helpful :)

@ChrisChV @sdaitzman Do you agree to move forward with the current version?

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@rpenido 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.